-
Notifications
You must be signed in to change notification settings - Fork 453
chore: add timestamp range to flight meta #7513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add timestamp range to flight meta #7513
Conversation
Add optional start_timestamp and end_timestamp fields to DoPutMetadata to support time-windowed batch operations in the Flight DoPut API. Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
6b759e5 to
4c6c27a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds timestamp range metadata fields to the Flight protocol's DoPut operation to support better ingestion performance optimization. The changes allow clients to optionally specify time windows for data batches during Flight-based ingestion.
Key changes:
- Adds optional
start_timestampandend_timestampfields toDoPutMetadatawith corresponding getter methods - Makes several
MemtableStatsfields public (estimated_bytes, time_range, max_sequence, series_count)
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/common/grpc/src/flight/do_put.rs | Adds optional timestamp range fields to DoPutMetadata for time-windowed batch metadata |
| src/mito2/src/memtable.rs | Changes visibility of MemtableStats fields from private to public |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/common/grpc/src/flight/do_put.rs
Outdated
| pub fn start_timestamp(&self) -> Option<i64> { | ||
| self.start_timestamp | ||
| } | ||
|
|
||
| pub fn end_timestamp(&self) -> Option<i64> { | ||
| self.end_timestamp | ||
| } |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new timestamp fields (start_timestamp and end_timestamp) have getter methods but no setter methods or builder pattern. This makes the API incomplete - there's no way for clients to populate these fields after construction. Consider adding setter methods (e.g., with_start_timestamp, with_end_timestamp) following the builder pattern used elsewhere in the codebase, or provide a constructor that accepts these optional parameters.
fengjiachun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
c449ecb to
c0ee918
Compare
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
Add timestamp range to Flight DoPutMetadata to facilitate better ingestion performance.
PR Checklist
Please convert it to a draft if some of the following conditions are not met.